feat(sdk): route chat.agent transcript persistence through a TranscriptStorage seam - #4893
Conversation
|
WalkthroughThe change adds transcript storage with reducers, shadow-based diffs, pagination, cursors, and version 2 snapshot persistence. It adds injectable and production snapshot I/O paths with failure handling. Chat agent persistence now uses transcript changesets for loading, completed turns, actions, and errors. Mocks and integration tests support version 1 compatibility and version 2 message envelopes. Merge Risk: 🟡 Moderate · up to Continuation and failed-upload paths can persist incomplete or stale transcript data, and a snapshot transport security concern remains unresolved. These issues should be addressed before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the design and implementation in detail, but it omits the required issue reference, checklist, Testing section, Changelog section, and Screenshots section from the repository template. Full details: Docstring CoverageExplanation Docstring coverage is 63.16% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 11 files. (1 skipped: 1 too large.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4c91a95 to
a7e6626
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
49acce9 to
f5c6372
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
f5c6372 to
15f04f2
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
15f04f2 to
8ead6fd
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
8ead6fd to
85f3ff1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/trigger-sdk/src/v3/chatSnapshotIo.ts (1)
112-162: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPropagate snapshot write failures before advancing the shadow. When presigning or uploading fails,
writeChatSnapshotlogs and resolves.snapshotTranscriptStorage.savethen resolves, sosaveTranscriptassignstranscriptShadow = shadowfor data that was not persisted. Later changesets omit the failed diff, and no retry occurs if the run ends first. Reject the write so the shadow remains unchanged and the next save retries the full diff.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: c6686b9a-d738-4074-9091-864aedf8390d
📒 Files selected for processing (4)
packages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/test/transcript-storage.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (34)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: sdk-compat / Node.js 26.4 (warp-ubuntu-latest-x64-4x)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: sdk-compat / Node.js 24.18 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (9)
Always import from `@trigger.dev/sdk`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
We use vitest exclusively.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/transcript-storage.test.ts
**Prefer static imports over dynamic imports.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
Add crumbs as you write code — not just when debugging.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/transcript-storage.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
Use types over interfaces for TypeScript Avoid using enums; prefer string unions or const objects instead
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs Do not use high-cardinality attributes in OTEL metr...
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
packages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4893
File: packages/trigger-sdk/test/transcript-storage.test.ts:190-197
Timestamp: 2026-09-06T05:39:13.239Z
Learning: In `triggerdotdev/trigger.dev`, `packages/trigger-sdk/test/transcript-storage.test.ts` uses the existing `__setReadChatSnapshotImplForTests` and `__setWriteChatSnapshotImplForTests` hooks to test pure transcript reducer, diff, and storage-adapter behavior. The real snapshot I/O transport is covered separately by MinIO-backed webapp tests. Do not require testcontainers for this unit-test file.
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-09-06T05:56:52.840Z
Learning: In `triggerdotdev/trigger.dev` `packages/trigger-sdk/src/v3/transcriptStorage.ts`, a transcript entry saved with `final: false` must remain non-final when its `UIMessage` content is unchanged in later `diffTranscript()` calls. `TranscriptShadow.nonFinal` preserves this status. When the message content changes, `diffTranscript()` emits a `put` with final status so `reduceTranscriptChanges()` stores the entry as final.
📚 Learning: 2026-08-16T18:36:58.179Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4537
File: packages/trigger-sdk/test/normalizeKeyString.test.ts:1-2
Timestamp: 2026-08-16T18:36:58.179Z
Learning: For related SDK `chat.agent` tests in the Trigger.dev repository—including chat channels, handover, snapshot, and transport-event coverage—keep new test files under `packages/trigger-sdk/test/` rather than colocating them with the `packages/trigger-sdk/src/v3/` source files.
Applied to files:
packages/trigger-sdk/test/transcript-storage.test.ts
🪛 ast-grep (0.45.2)
packages/trigger-sdk/src/v3/ai.ts
[warning] 7035-7042: Avoid logging sensitive data
Context: logger.warn(
"chat.agent: snapshot write outside a turn failed; the change may not survive a continuation",
{
error: error instanceof Error ? error.message : String(error),
sessionId: sessionIdForSnapshot,
reason,
}
)
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
[warning] 7084-7087: Avoid logging sensitive data
Context: logger.warn("chat.agent: transcript load failed; continuing from the stream tail", {
error: error instanceof Error ? error.message : String(error),
sessionId: sessionIdForSnapshot,
})
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🔇 Additional comments (1)
packages/trigger-sdk/test/transcript-storage.test.ts (1)
44-339: LGTM!
85f3ff1 to
627899d
Compare
…ptStorage seam Introduces the TranscriptStorage interface (load/save over id-addressed changes: put, remove, truncateAfter, state) and makes the built-in snapshot writer its default implementation. The runtime keeps a shadow of the transcript it last saved and hands the storage the diff after every turn, failed turn and history-changing action, together with the stream cursors the next boot resumes from. The default storage reduces each changeset onto an in-memory copy and rewrites the blob as version 2, so a turn still costs one PUT and no GET, and the boot read goes through load. The snapshot read/write helpers move to their own module so the storage can import them without a cycle; the test seams keep their import path. The mock chat agent harness now reports version 2 snapshots and accepts either version as a seed.
627899d to
5b764c7
Compare
2 participants
Summary
Introduces the
TranscriptStorageseam insidechat.agentand makes the built-in snapshot writer its default implementation. No public option yet; the default path behaves as before, apart from the blob now being written as version 2.Design
A storage has
load(called once when a run boots to continue a conversation) andsave(called after every turn, failed turn and history-changing action).savereceives a changeset of id-addressed operations:put(upsert by message id),remove,truncateAfterandstate, plus the stream cursors the next boot resumes from.The runtime keeps a shadow of the transcript it last handed to
save(ids in order plus a fingerprint per message) and diffs the accumulator against it. A changed message in the common prefix is an in-placeput; anything past the prefix is onetruncateAfteron the last common id followed byputs in order. Applying the result reproduces the accumulator exactly for any edit, and the common cases come out as the natural operations: a turn is twoputs, an undo is onetruncateAfter, a regenerate is atruncateAfterand aput. The shadow only advances whensaveresolves, so a failed save is folded into the next changeset; every operation is idempotent, so a retried changeset converges.Every changeset also carries the whole transcript as it stands after the changes (entries with a
finalflag, plus the runtime'sstate). A row-per-message store applies the changes; a store that keeps the conversation as one document writes the transcript as-is and needs no state of its own between saves. The default storage is the second kind: it serialises the transcript and rewrites the blob, so a turn still costs one PUT and no GET, and nothing is held in memory between saves. The snapshot read and write helpers move to their own module so the storage can import them without a cycle; the test seams keep their import path.